Add Aave Lending Deposit and Withdrawal With Delegation#136
Add Aave Lending Deposit and Withdrawal With Delegation#136
Conversation
hanzel98
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
It is pending to validate if we prefer the simple If we decide that we want both options, then we need to unify the code to avoid code duplication and also apply the same approach to the other adapters. |
There was a problem hiding this comment.
Bug: Withdrawal Events Emit Incorrect Amount
The withdrawByDelegation and withdrawByDelegationOpenEnded functions incorrectly emit the WithdrawExecuted event with the requested _amount parameter. The aavePool.withdraw() function returns the actual amount withdrawn, which can differ from the requested amount (e.g., when type(uint256).max is used for full withdrawal). This leads to inaccurate event data.
src/helpers/AaveAdapter.sol#L186-L189
delegation-framework/src/helpers/AaveAdapter.sol
Lines 186 to 189 in 67a37fb
src/helpers/AaveAdapter.sol#L217-L220
delegation-framework/src/helpers/AaveAdapter.sol
Lines 217 to 220 in 67a37fb
Bug: Allowance Overflow in SafeIncreaseAllowance
The _ensureAllowance function can cause an arithmetic overflow. If the current allowance is non-zero but insufficient, calling safeIncreaseAllowance with type(uint256).max will attempt currentAllowance + type(uint256).max, which overflows uint256 and reverts the transaction (due to Solidity 0.8+ overflow checks). This logical flaw, while unlikely in practice, can be resolved by using safeApprove to set the allowance directly to type(uint256).max or by calculating the precise increase amount.
src/helpers/AaveAdapter.sol#L93-L99
delegation-framework/src/helpers/AaveAdapter.sol
Lines 93 to 99 in 67a37fb
Was this report helpful? Give feedback by reacting with 👍 or 👎
|
This PR needs an RPC URL secret on github to work |
| if (allowance_ < _amount) { | ||
| _token.safeIncreaseAllowance(address(aavePool), _amount); | ||
| } | ||
| } |
There was a problem hiding this comment.
_ensureAllowance over-approves by adding instead of setting allowance
Medium Severity
_ensureAllowance calls safeIncreaseAllowance(spender, _amount) which adds _amount to the current allowance, resulting in allowance_ + _amount. The intent is to ensure the allowance is at least _amount, so the increase should be _amount - allowance_, or forceApprove could set it directly to _amount. While this works when the starting allowance is zero (normal flow), any residual allowance causes over-approval and risks arithmetic overflow with large _amount values. The same issue exists in MorphoAdapter._ensureAllowance.
Additional Locations (1)
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| if (_token == address(0)) revert InvalidZeroAddress(); | ||
|
|
||
| // Root delegator is the original token owner (last in the delegation chain) | ||
| address rootDelegator_ = _delegations[1].delegator; |
There was a problem hiding this comment.
Wrong root delegator in withdrawal for multi-hop chains
High Severity
withdrawByDelegation uses hardcoded _delegations[1].delegator for rootDelegator_, but its length check (< 2) permits chains longer than 2. In contrast, _executeSupplyByDelegation correctly uses _delegations[length_ - 1].delegator. For a 3-element chain, the withdrawn funds from aavePool.withdraw would be sent to an intermediate delegator instead of the actual root token owner, causing loss of funds.
Additional Locations (1)
| "test/helpers/AaveLending.t.sol" | ||
| # Add more shanghai tests here in the future | ||
| # "test/helpers/AnotherShanghaiTest.t.sol" | ||
| ) |
There was a problem hiding this comment.
MorphoLending test missing from Shanghai coverage configuration
Low Severity
MorphoLending.t.sol declares forge-config: default.evm_version = "shanghai" but is not listed in the SHANGHAI_TESTS array in coverage.sh. Only AaveLending.t.sol is listed. This means MorphoLending.t.sol won't be excluded from the London EVM coverage run and won't be included in the Shanghai run, potentially causing coverage failures or incorrect coverage data.
Additional Locations (1)
2 participants
What?
Why?
How?
Notes
Note
Medium Risk
Adds new adapter smart contracts that custody tokens briefly and invoke external DeFi protocols (Aave/Morpho) via delegation redemption, which is higher-risk than typical changes despite being additive and well-tested on mainnet forks.
Overview
Adds a new Delegation Adapters concept to the docs, including a new
documents/Adapters.mddescribing current adapters and linking it fromREADME.md.Introduces an
AaveAdapterhelper contract that redeems delegation chains to pull ERC-20/aToken transfers into the adapter and then performs Aavesupply/withdraw, plus an owner-onlywithdrawEmergencyrecovery path. Adds supporting Aave pool interface (IAavePool) and aDeployAaveAdapterFoundry deployment script.Adds a
MorphoAdapterhelper contract andIMorphoVaultinterface to perform ERC-4626deposit/redeemvia delegation redemption (including self-calls executed throughDelegationManager), along with extensive mainnet-fork tests for both Aave (AaveLending.t.sol) and Morpho (MorphoLending.t.sol). Updatesscript/coverage.shto run/merge coverage across EVM versions and include Shanghai-only tests.Written by Cursor Bugbot for commit 56357fb. This will update automatically on new commits. Configure here.